Skip to content

ast: Allow non-infix and()/or() set built-in calls - #9012

Open
johanfylling wants to merge 2 commits into
open-policy-agent:mainfrom
johanfylling:and_or/allow_set_built-in_calls
Open

ast: Allow non-infix and()/or() set built-in calls#9012
johanfylling wants to merge 2 commits into
open-policy-agent:mainfrom
johanfylling:and_or/allow_set_built-in_calls

Conversation

@johanfylling

Copy link
Copy Markdown
Contributor

The and/or keywords are infix operators, so when they appear in a term position immediately followed by an opening parens (() there is no ambiguity that we're dealing with a function call and not a logical operation.

Being permissive in this position allow users to keep calling the named form (and()/or()) of the set built-ins (&/| infixes).

Note: calling the named functions, as opposed to using the infix form, is likely very rare, but we can't guarantee this form is never used.

The `and`/`or` keywords are infix operators, so when they appear in a term position immediately followed by an opening parens (`(`) there is no ambiguity that we're dealing with a function call and not a logical operation.

Being permissive in this position allow users to keep calling the named form (`and()`/`or()`) of the set built-ins (`&`/`|` infixes).

Note: calling the named functions, as opposed to using the infix form, is likely very rare, but we can't guarantee this form is never used.
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Conflicts:
	v1/ast/parser_logical_test.go
// `z and (1 + 2) > 3` is accepted -- the same shape is plain v1 syntax after
// `not`, see TestParseLogical_ParenNot.
{
note: "rhs operand, leading part wrapped",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean to delete these test cases? They were added in github.com//pull/9006 so I was thinking maybe merge conflict accident?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants